Core: Pass REST auth manager to S3 signer by 010Soham · Pull Request #2846 · apache/iceberg-python

010Soham · 2025-12-19T11:50:40Z

What does this change do?

RestCatalog now passes its AuthManager into FileIO so downstream components can reuse a live token.
S3V4RestSigner now calls the AuthManager’s auth_header() when no static token is provided, ensuring the signer gets a fresh bearer token.
Added a unit test to verify the signer pulls the Authorization header from an AuthManager.

Why is this needed?

After the AuthManager refactor, the signer no longer received a token, causing remote signing to 401 for REST catalog users (e.g., Lakekeeper/MinIO). This restores token propagation and refresh.

How was this tested?

make lint
make test
uv run python -m pytest tests/io/test_fsspec.py -k auth_manager -v

Closes #2544

Fokko · 2025-12-22T16:06:14Z

Thanks a lot @010Soham for picking this up, a lot of folks are eagerly waiting for this 👍

@c-thiel Are you able to check if this fixes the issue with the server signed urls?

kevinjqliu

LGTM! Thanks for the fix. I commented on the original issue (#2544) to see if anyone can verify the solution.

I'll also try to reproduce the issue and try out this fix locally

kevinjqliu · 2025-12-24T02:01:33Z

pyiceberg/io/fsspec.py

        if token := self.properties.get(TOKEN):
-            signer_headers = {"Authorization": f"Bearer {token}"}
+            auth_header = f"Bearer {token}"


nit: maybe we can get rid of accessing TOKEN through properties here and just standardize on using the auth manager.

This may be out of scope for this bug-fix PR, but it looks like we’re tightly coupling AuthManager and authentication tokens which are RestCatalog concepts into FileIO, which should be Catalog type agnostic.

It might be worth revisiting this design in more detail in the future to ensure we don’t introduce fallback logic that’s driven by configuration properties rather than clearer separation of concerns

yea great point. i think it would be better to split out the "rest signer" from fileio. there's a good example already in the REST catalog,

iceberg-python/pyiceberg/catalog/rest/__init__.py

Lines 409 to 459 in a99dcad

def _init_sigv4(self, session: Session) -> None:

from urllib import parse

import boto3

from botocore.auth import SigV4Auth

from botocore.awsrequest import AWSRequest

from requests import PreparedRequest

from requests.adapters import HTTPAdapter

class SigV4Adapter(HTTPAdapter):

def __init__(self, **properties: str):

super().__init__()

self._properties = properties

self._boto_session = boto3.Session(

region_name=get_first_property_value(self._properties, AWS_REGION),

botocore_session=self._properties.get(BOTOCORE_SESSION),

aws_access_key_id=get_first_property_value(self._properties, AWS_ACCESS_KEY_ID),

aws_secret_access_key=get_first_property_value(self._properties, AWS_SECRET_ACCESS_KEY),

aws_session_token=get_first_property_value(self._properties, AWS_SESSION_TOKEN),

)

def add_headers(self, request: PreparedRequest, **kwargs: Any) -> None: # pylint: disable=W0613

credentials = self._boto_session.get_credentials().get_frozen_credentials()

region = self._properties.get(SIGV4_REGION, self._boto_session.region_name)

service = self._properties.get(SIGV4_SERVICE, "execute-api")

url = str(request.url).split("?")[0]

query = str(parse.urlsplit(request.url).query)

params = dict(parse.parse_qsl(query))

# remove the connection header as it will be updated after signing

del request.headers["connection"]

aws_request = AWSRequest(

method=request.method, url=url, params=params, data=request.body, headers=dict(request.headers)

)

SigV4Auth(credentials, service, region).add_auth(aws_request)

original_header = request.headers

signed_headers = aws_request.headers

relocated_headers = {}

# relocate headers if there is a conflict with signed headers

for header, value in original_header.items():

if header in signed_headers and signed_headers[header] != value:

relocated_headers[f"Original-{header}"] = value

request.headers.update(relocated_headers)

request.headers.update(signed_headers)

session.mount(self.uri, SigV4Adapter(**self.properties))

it might also be easier to just pass in the request Session from the REST catalog to the Signer. So we dont need to recreate the auth header directly

but again, we can refactor this after the bug fix :)

agreed - just leaving a comment so we don't forget 🙂

in case we forget, #2862

that sounds good, lets address the cleaner design in #2862

pyiceberg/io/fsspec.py

pyiceberg/catalog/__init__.py

kevinjqliu · 2025-12-24T02:25:31Z

i was able to verify this locally by using the amazing repro script from @martyngigg
See #2544 (comment)

pyiceberg/io/fsspec.py

fboerman · 2026-01-02T14:42:26Z

I ran into the issue linked and can confirm using this PR with pyiceberg[s3fs,rest-sigv4] @ git+https://github.com/apache/iceberg-python@refs/pull/2846/head works. would be great to get this merged

martyngigg · 2026-01-05T09:28:20Z

I can also confirm that the solution works for me too. Thanks for the fix!

pyiceberg/catalog/rest/__init__.py

Fokko

This looks good to me as well.

@010Soham are you able to do a pass on the pending comments? It would be good to get this in and start another release.

Thanks everyone here for testing locally to see if this fixes the issue, much appreciated 🙌

010Soham · 2026-01-06T07:32:43Z

Thanks everyone for your suggestions and testing things out
@Fokko
I went through all the pending comments and addressed them:

S3V4RestSigner now prefers auth_manager.auth_header() (called directly) and falls back to token only if no auth manager is present.
Moved AUTH_MANAGER to auth.py and updated imports.

All CI checks are green now

kevinjqliu

LGTM! Thank you!

I reran this integration once again locally ✅ ✅ ✅
#2544 (comment)

kevinjqliu · 2026-01-16T00:26:25Z

pyiceberg/catalog/rest/__init__.py

+            self._auth_manager = self._create_legacy_oauth2_auth_manager(session)
+            session.auth = AuthManagerAdapter(self._auth_manager)


nit: take session.auth = AuthManagerAdapter(self._auth_manager) part out of the if/else branch since its the same logic

kevinjqliu · 2026-01-16T00:39:06Z

leaving this open for a few more reviews, i plan to merge this first thing tmr 😄
super exciting, we'll be able to release 0.11 soon after this fix

geruh

LGTM!

kevinjqliu · 2026-01-17T19:46:16Z

Thanks for the contribution @010Soham and thank you @sungwy @Fokko @geruh for the review!

010Soham · 2026-01-20T14:23:27Z

Glad it was helpful @kevinjqliu 😄, eagerly waiting for the 0.11.0 release and build on from the learnings 0.10.0

010Soham and others added 3 commits December 19, 2025 11:49

Fix REST signer to use catalog auth manager

203bbb4

Merge branch 'main' into fix-2544-rest-signer-token

d7d57ea

Apply ruff format after merge

a99dcad

Fokko added this to the PyIceberg 0.11.0 milestone Dec 22, 2025

kevinjqliu mentioned this pull request Dec 24, 2025

FileIO properties missing TOKEN after AuthManager refactor #2544

Closed

3 tasks

kevinjqliu approved these changes Dec 24, 2025

View reviewed changes

kevinjqliu reviewed Dec 24, 2025

View reviewed changes

pyiceberg/catalog/__init__.py Outdated Show resolved Hide resolved

kevinjqliu mentioned this pull request Dec 24, 2025

feat: refactor rest signer in fileio #2862

Open

geruh reviewed Dec 25, 2025

View reviewed changes

pyiceberg/io/fsspec.py Show resolved Hide resolved

Merge branch 'main' into fix-2544-rest-signer-token

72be520

Fokko reviewed Jan 5, 2026

View reviewed changes

pyiceberg/catalog/rest/__init__.py Show resolved Hide resolved

Fokko approved these changes Jan 5, 2026

View reviewed changes

address review feedback:prefer auth manager,move constant

570ba7c

kevinjqliu approved these changes Jan 16, 2026

View reviewed changes

geruh approved these changes Jan 16, 2026

View reviewed changes

Merge branch 'main' into fix-2544-rest-signer-token

92bbc7a

kevinjqliu merged commit ba65619 into apache:main Jan 17, 2026
10 checks passed

kevinjqliu mentioned this pull request Jan 25, 2026

S3 Remote Signing no token Refresh #2823

Closed

3 tasks

fboerman mentioned this pull request Jan 26, 2026

signing error when using s3 alternatives #2790

Closed

3 tasks

	def _init_sigv4(self, session: Session) -> None:
	from urllib import parse

	import boto3
	from botocore.auth import SigV4Auth
	from botocore.awsrequest import AWSRequest
	from requests import PreparedRequest
	from requests.adapters import HTTPAdapter

	class SigV4Adapter(HTTPAdapter):
	def __init__(self, **properties: str):
	super().__init__()
	self._properties = properties
	self._boto_session = boto3.Session(
	region_name=get_first_property_value(self._properties, AWS_REGION),
	botocore_session=self._properties.get(BOTOCORE_SESSION),
	aws_access_key_id=get_first_property_value(self._properties, AWS_ACCESS_KEY_ID),
	aws_secret_access_key=get_first_property_value(self._properties, AWS_SECRET_ACCESS_KEY),
	aws_session_token=get_first_property_value(self._properties, AWS_SESSION_TOKEN),
	)

	def add_headers(self, request: PreparedRequest, **kwargs: Any) -> None: # pylint: disable=W0613
	credentials = self._boto_session.get_credentials().get_frozen_credentials()
	region = self._properties.get(SIGV4_REGION, self._boto_session.region_name)
	service = self._properties.get(SIGV4_SERVICE, "execute-api")

	url = str(request.url).split("?")[0]
	query = str(parse.urlsplit(request.url).query)
	params = dict(parse.parse_qsl(query))

	# remove the connection header as it will be updated after signing
	del request.headers["connection"]

	aws_request = AWSRequest(
	method=request.method, url=url, params=params, data=request.body, headers=dict(request.headers)
	)

	SigV4Auth(credentials, service, region).add_auth(aws_request)
	original_header = request.headers
	signed_headers = aws_request.headers
	relocated_headers = {}

	# relocate headers if there is a conflict with signed headers
	for header, value in original_header.items():
	if header in signed_headers and signed_headers[header] != value:
	relocated_headers[f"Original-{header}"] = value

	request.headers.update(relocated_headers)
	request.headers.update(signed_headers)

	session.mount(self.uri, SigV4Adapter(**self.properties))

		self._auth_manager = self._create_legacy_oauth2_auth_manager(session)
		session.auth = AuthManagerAdapter(self._auth_manager)

Conversation

010Soham commented Dec 19, 2025

Uh oh!

Fokko commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinjqliu left a comment

Choose a reason for hiding this comment

Uh oh!

kevinjqliu Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

sungwy Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinjqliu Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

sungwy Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

kevinjqliu Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

010Soham Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kevinjqliu commented Dec 24, 2025

Uh oh!

Uh oh!

fboerman commented Jan 2, 2026

Uh oh!

martyngigg commented Jan 5, 2026

Uh oh!

Uh oh!

Fokko left a comment

Choose a reason for hiding this comment

Uh oh!

010Soham commented Jan 6, 2026

Uh oh!

kevinjqliu left a comment

Choose a reason for hiding this comment

Uh oh!

kevinjqliu Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

kevinjqliu commented Jan 16, 2026

Uh oh!

geruh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kevinjqliu commented Jan 17, 2026

Uh oh!

010Soham commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Fokko commented Dec 22, 2025 •

edited

Loading

sungwy Dec 24, 2025 •

edited

Loading

010Soham commented Jan 20, 2026 •

edited

Loading